feat: "Connect with Tolgee" OAuth login for in-context editing - #39

src/background/background.ts (1)

75-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Log the messaging failure instead of discarding it.

The catch swallows every error. If the content script is not present, login appears to succeed but no credentials reach the page, and nothing records why.

Keep the non-throwing behavior, and log the reason.

🛠️ Proposed change

   await browser.tabs
     .sendMessage(tabId, { type: 'SET_CREDENTIALS', data })
-    .catch(() => undefined);
+    .catch((e) =>
+      console.debug('[tolgee-oauth] credential injection skipped', tabId, e)
+    );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/background/background.ts` around lines 75 - 77, Update the sendMessage
error handler in the background messaging flow to capture the caught error and
log its reason, while preserving the existing non-throwing behavior and
undefined fallback.

src/oauth/tokenStore.ts (1)

7-12: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Document the storage choice for refresh tokens.

browser.storage.local persists unencrypted on disk and survives browser restarts. It holds the refresh token here. That is a deliberate trade-off for long-lived sessions, but it differs from browser.storage.session, which stays in memory.

Record the reason in the comment, and confirm the platform enforces an absolute refresh-token lifetime so a stale on-disk token cannot be replayed indefinitely.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/oauth/tokenStore.ts` around lines 7 - 12, Update the comment above
saveSession to document that browser.storage.local stores refresh tokens
unencrypted on disk across browser restarts as a deliberate long-lived-session
trade-off versus browser.storage.session, and state the platform’s enforced
absolute refresh-token lifetime that prevents indefinite replay of stale tokens.

src/constants.ts (1)

14-25: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider narrowing the requested scopes.

The access token is written into page-accessible storage (AUTH_TOKEN_LOCAL_STORAGE). Any script running on that page can read it. Backend intersection with user permissions prevents privilege escalation, but it does not limit what a hostile page script can do with the token inside the user's own rights. screenshots.delete and keys.edit are destructive.

Request only the scopes the in-context editor actually calls, or request scopes incrementally.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/constants.ts` around lines 14 - 25, Update OAUTH_SCOPES to include only
the permissions directly required by the in-context editor, removing destructive
scopes such as screenshots.delete and keys.edit unless their corresponding
operations are actually invoked. If those operations are needed conditionally,
request their scopes incrementally rather than in the default token scope set.

src/oauth/oauthClient.ts (1)

31-39: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Do not put the raw response body in the error, and add a request timeout.

The thrown message embeds the full token-endpoint body. src/background/background.ts (Lines 45-46) logs that error and returns String(e) to the popup. A token endpoint can echo request parameters in an error body, so a refresh token or authorization code can reach the console and the popup.

fetch also has no AbortSignal. apiUrl is user-supplied, so a non-responsive host stalls the login and refresh paths.

🛠️ Proposed fix

   const res = await fetch(`${base}/oauth2/token`, {
     method: 'POST',
     headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
     body: new URLSearchParams(params),
+    signal: AbortSignal.timeout(15_000),
   });
   if (!res.ok) {
-    const body = await res.text().catch(() => '');
-    throw new Error(`Tolgee token endpoint returned ${res.status}: ${body}`);
+    const body = await res.json().catch(() => null);
+    const reason = body && typeof body.error === 'string' ? body.error : '';
+    throw new Error(
+      `Tolgee token endpoint returned ${res.status}${reason ? `: ${reason}` : ''}`
+    );
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/oauth/oauthClient.ts` around lines 31 - 39, Update the token request in
the OAuth client’s fetch flow to avoid including the raw response body in thrown
errors; report only the safe HTTP status or a generic failure message. Add an
AbortSignal-based timeout to the fetch request, using the project’s established
timeout convention if available, so user-supplied apiUrl hosts cannot stall
login or refresh indefinitely.

src/popup/storage.ts (1)

25-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reuse the shared Values type instead of the local intersection.

src/popup/tools.ts already declares Values with authToken and projectId. This file keeps a second local Values and then widens it with & { authToken?: string }. The two definitions can drift, and a reader cannot tell which one is authoritative. Import the type from ./tools and delete the local copy.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/popup/storage.ts` around lines 25 - 39, Update storeValues to import and
use the shared Values type from ./tools directly, removing the local Values
definition and the authToken intersection. Preserve the existing optional values
handling and storage behavior.

src/popup/useDetectorForm.tsx (1)

245-296: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Add the resolution inputs to the dependency array.

The effect reads libConfig?.config?.projectId, checkableValues.apiUrl, and checkableValues.authToken, but it only depends on state.credentialsCheck. If the page reports a new projectId through TOLGEE_CONFIG_LOADED while credentialsCheck stays the same object, the popup keeps the previously resolved project. Add the read values to the dependency list.

♻️ Proposed dependency change

-  }, [state.credentialsCheck]);
+  }, [
+    state.credentialsCheck,
+    (libConfig?.config as { projectId?: number | string } | undefined)
+      ?.projectId,
+    checkableValues?.apiUrl,
+    checkableValues?.authToken,
+  ]);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/popup/useDetectorForm.tsx` around lines 245 - 296, Update the dependency
array of the project-resolution useEffect to include
libConfig?.config?.projectId and the read checkableValues.apiUrl and
checkableValues.authToken values alongside state.credentialsCheck, so resolution
reruns when any input changes.

src/popup/sendToBackground.ts (1)

4-6: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Handle sendMessage rejections in this helper.

browser.runtime.sendMessage rejects when the service worker does not answer, for example after a worker restart or when the message channel closes. The callers (handleConnect in src/popup/TolgeeDetector.tsx and onLibConfigChange in src/popup/useDetectorForm.tsx) do not catch the rejection, so the popup produces an unhandled rejection and the user sees no feedback. Return a normalized error result here, or add catch at every call site.

♻️ Proposed helper change

-export const sendToBackground = async (type: string, data?: any) => {
-  return browser.runtime.sendMessage({ type, data });
-};
+export const sendToBackground = async (type: string, data?: any) => {
+  try {
+    return await browser.runtime.sendMessage({ type, data });
+  } catch (e) {
+    console.error(e);
+    return { error: String(e) };
+  }
+};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/popup/sendToBackground.ts` around lines 4 - 6, Update sendToBackground to
catch rejections from browser.runtime.sendMessage and return a normalized error
result instead of allowing the promise rejection to propagate. Preserve the
existing message payload and successful response behavior, using the helper’s
return contract so callers such as handleConnect and onLibConfigChange receive a
consistent failure result.

src/popup/reducer.test.ts (1)

1-207: LGTM!

src/popup/tools.test.ts (1)

1-123: LGTM!

vitest.config.ts (1)

1-10: LGTM!

package.json (1)

19-21: LGTM!

Also applies to: 51-52

.github/workflows/test.yml (1)

33-35: LGTM!

src/constants.ts (2)

4-11: LGTM!

---

12-13: 🩺 Stability & Availability | ⚡ Quick win

Verify the refresh skew against the backend access-token lifetime.

OAUTH_REFRESH_SKEW_MS is 60s, but the background refresh alarm in src/background/background.ts (Line 82) runs every 10 minutes. If the backend issues access tokens with a lifetime under about 11 minutes, a token can expire in the page before the next alarm fires. The page-side token is only rotated by that alarm.

Confirm the platform access-token TTL, then align the alarm period with it (for example, period ≤ TTL/2) or derive the period from expiresAt.

src/oauth/pkce.ts (1)

3-23: LGTM!

src/oauth/oauthClient.ts (1)

84-96: LGTM!

src/oauth/tokenStore.ts (1)

26-31: LGTM!

manifest.json (2)

5-5: 🩺 Stability & Availability

Confirm the pinned key matches the Web Store item.

The key field pins the extension ID so identity.getRedirectURL() stays stable, which the OAuth redirect URI depends on. The value is a public key, so publishing it is safe.

The key must match the one the Chrome Web Store assigned to this item. If it does not match, the store-installed build gets a different ID and the pre-registered redirect URI stops matching. Confirm the value against the store listing, and confirm Firefox builds do not need this field.

---

18-18: LGTM!

src/background/background.ts (2)

4-16: LGTM!

---

100-113: 🩺 Stability & Availability

⚠️ Unverified finding
Sandbox verification was unavailable.

Verify the content script rejects tokens for a non-matching apiUrl.

browser.tabs.query({}) returns every tab. This function then sends the OAuth access token to each one. The token reaches every tab that runs the content script, including tabs where the user never applied Tolgee credentials.

The design depends on the content script comparing data.apiUrl against the backend already applied in that tab, and discarding the message otherwise. src/content/contentScript.ts is not in this review context, so that guard is not confirmed here.

Confirm the guard exists and compares origins, not raw strings. Alternatively, track which tabs received credentials and send only to those.

src/content/contentScript.ts (2)

4-6: LGTM!

Also applies to: 18-37

---

84-110: LGTM!

src/popup/tools.ts (1)

5-16: LGTM!

Also applies to: 18-19, 25-32, 38-61

src/popup/storage.ts (1)

7-11: LGTM!

Also applies to: 66-67

src/popup/reducer.ts (2)

1-72: LGTM!

---

156-244: LGTM!

src/popup/useDetectorForm.tsx (1)

6-17: LGTM!

Also applies to: 75-90, 133-192

src/popup/TolgeeDetector.tsx (3)

1-37: LGTM!

Also applies to: 48-68

---

122-205: LGTM!

Also applies to: 207-243, 269-281

---

284-358: LGTM!

Also applies to: 375-421, 423-485

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes